Skip to content

Comments

Козлова Валерия#13

Open
OnaHochetNaMars wants to merge 21 commits intourfu-2016:masterfrom
OnaHochetNaMars:master
Open

Козлова Валерия#13
OnaHochetNaMars wants to merge 21 commits intourfu-2016:masterfrom
OnaHochetNaMars:master

Conversation

@OnaHochetNaMars
Copy link

@OnaHochetNaMars OnaHochetNaMars commented Nov 28, 2016

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍏 Пройден линтинг и базовые тесты

@honest-hrundel
Copy link

🍏 Пройден линтинг и базовые тесты

index.html Outdated
<div class="content">
<div class="img">
<label for="card-2">
<img src="images/img2.jpg" alt="">
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❗️ alt не должен быть пустым. Изображение можно назвать понятней)

index.html Outdated
<input type="radio" name="gallery" id="card-6" class="card-6">
<input type="radio" name="gallery" id="card-7" class="card-7">
<br>

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❗️ Хочется семантичных тегов

index.html Outdated
<div class="card card-6">
<input type="checkbox" id="show-recipe-6">
<div class="content">
<div class="img">
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❗️ Можно придумать более конкретные названия для классов, чем content и img.

index.html Outdated
<h1>Название рецепта</h1>
<div>
<div class="ingredients">
Ингридиенты
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ингредиенты

index.html Outdated
</div>
<div class="recipe-description">
<label class="close" for="show-recipe-5">X</label>
<h1>Название рецепта</h1>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❗️ h1 может быть на странице только 1 и должен быть заголовком всей страницы

index.css Outdated
transition: all .4s ease-in-out;
box-sizing: border-box;
padding-top: 30%;
z-index: -1;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А зачем z-index, если и так opacity: 0?

index.css Outdated
z-index: 10;
}

body
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❗️ Оправдано ли использование position: relative здесь?

index.css Outdated
z-index: 11;
}

.recipe-description
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❗️ Повторяющийся селектор

index.css Outdated

.recipe-description
{
display: flex;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❗️ Хм, зачем ты используешь flex, если сам крестик направо перемещаешь с помощью text-align?

.ingredients,
.steps
{
width: 50%;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❗️ Если ты используешь флекс у родительского элемента, то располагать элементы внутри лучше с помощью него

@Dodo888
Copy link

Dodo888 commented Nov 30, 2016

❗️ По условию задачи, чем дальше фотография от активной - тем меньше должен быть ее поворот. Сейчас у всех неактивных изображений поворот одинаковый
image

@Dodo888
Copy link

Dodo888 commented Nov 30, 2016

❗️ Рецепт должен открываться при нажатии на активное изображение, а не только на надпись "РЕЦЕПТ" внутри него

@Dodo888
Copy link

Dodo888 commented Nov 30, 2016

❗️ Отсутствует анимация закрытия карточки

@Dodo888
Copy link

Dodo888 commented Nov 30, 2016

❗️ Одна карточка превращается в 3. Одна осталась на месте, остальные 2 (с изображением и рецептом) начали переворачиваться, но видно, что они разные по размерам. Необходимо, чтобы визуально карточка всегда была одна
image

@Dodo888
Copy link

Dodo888 commented Nov 30, 2016

❗️ Давай карточку рецепта тоже красиво сверстаем, сейчас она выглядит нерепрезентативно. Не хватает отступов, красивого крестика, каких-нибудь разделителей, шрифтов (например, как в макете из условия)

@Dodo888
Copy link

Dodo888 commented Nov 30, 2016

В целом здорово, но есть недочеты

@Dodo888
Copy link

Dodo888 commented Nov 30, 2016

🍅

@honest-hrundel
Copy link

🍏 Пройден линтинг и базовые тесты

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍏 Пройден линтинг и базовые тесты

@honest-hrundel
Copy link

🍏 Пройден линтинг и базовые тесты

@honest-hrundel
Copy link

🍏 Пройден линтинг и базовые тесты

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍅 Не пройден линтинг или базовые тесты

@honest-hrundel
Copy link

🍏 Пройден линтинг и базовые тесты

@OnaHochetNaMars
Copy link
Author

🍏

<input class="show-recipe" type="checkbox" id="show-recipe-5">
<input class="show-recipe" type="checkbox" id="show-recipe-6">
<input class="show-recipe" type="checkbox" id="show-recipe-7">
<div class="card card-1">
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❗️ Можно все же использовать семантичные теги main, article, section

index.html Outdated
<input type="checkbox" id="show-recipe-3">
<div class="content">
<div class="img">
<label for="card-3">
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

И все-таки?

@Dodo888
Copy link

Dodo888 commented Dec 3, 2016

❗️ Иногда при наведении появляется затемнение, но не появляется кнопка "Рецепт"
image

@Dodo888
Copy link

Dodo888 commented Dec 3, 2016

❗️ При анимации закрытия карточка мгновенно становится маленькой, потом переворачивается. Уменьшение тоже должно происходить плавно

@Dodo888
Copy link

Dodo888 commented Dec 3, 2016

❗️ При открытой карточке должно появляться затемнение, задний фон должен быть неактивным.
image

@Dodo888
Copy link

Dodo888 commented Dec 3, 2016

❗️ В Firefox карточка не открывается
image

@Dodo888
Copy link

Dodo888 commented Dec 3, 2016

❗️ В Edge при открытой карточке появляется вертикальный скролл
image

@Dodo888
Copy link

Dodo888 commented Dec 3, 2016

В Edge проблема со шрифтом (в Chrome все хорошо)
image

@Dodo888
Copy link

Dodo888 commented Dec 3, 2016

Карточка теперь выглядит очень классно) Оставил замечания

@Dodo888
Copy link

Dodo888 commented Dec 3, 2016

🚀

@honest-hrundel honest-hrundel assigned msmirnov and unassigned Dodo888 Dec 3, 2016
@msmirnov
Copy link

msmirnov commented Dec 5, 2016

Когда карточка закрывается, то происходят какие-то рывки и анимация совсем не похожа на анимацию открытия.

@msmirnov
Copy link

msmirnov commented Dec 5, 2016

При переключении центральная карточка зачем-то получает hover, хотя я мышку на неё не наводил. Вводит в заблуждение.
screencast 2016-12-05 23-23-23

@msmirnov
Copy link

msmirnov commented Dec 5, 2016

🍅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants